Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(avc/slice): entire NALU consumed as slice header #272

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

thejoeejoee
Copy link
Contributor

@thejoeejoee thejoeejoee commented Sep 26, 2023

  • fix forloops in avc/slice header parser, where break doesn't break outer parser loop

@thejoeejoee thejoeejoee changed the title fix(avc/slice): breaks doesn't stop parser loops fix(avc/slice): entire NALU consumed as slice header Sep 26, 2023
Copy link
Collaborator

@tobbee tobbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @thejoeejoee,

thanks for your PR. It looks fine to me.

How did you find the problem? Did you run the code towards an asset or by just reading the code? If you have some relevant test material it would be great to add a small bit of it.

@thejoeejoee
Copy link
Contributor Author

Exactly, I was doing some experiments with encryption and parsed SliceHeader.Size was a few kilos. I just included the specific asset where ParseSliceHeader ate entire NALU and now it's fixed.

Copy link
Collaborator

@tobbee tobbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to get a test with small video NALUs!


func TestParseSliceHeader_TwoFrames(t *testing.T) {
wantedIdrHdr := SliceHeader{SliceType: SLICE_I, IDRPicID: 1, SliceQPDelta: 8, Size: 5}
wantedNonIdrHdr := SliceHeader{SliceType: SLICE_P, FrameNum: 1, ModificationOfPicNumsIDC: 3, SliceQPDelta: 13, Size: 5, NumRefIdxActiveOverrideFlag: true, RefPicListModificationL0Flag: true}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. This line is 194 characters long, so it exceeds the maximum line length of 140 configured in .golangci.yml.
Just break it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed that, fixed

@tobbee tobbee merged commit 2527ed1 into Eyevinn:master Sep 27, 2023
@tobbee
Copy link
Collaborator

tobbee commented Sep 27, 2023

Great! Thanks for your contribution!

@thejoeejoee thejoeejoee deleted the fix-avc-slice-header branch October 2, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants